-
Notifications
You must be signed in to change notification settings - Fork 151
Fix: Generate minified CSS for frontend-subscriptions and ai-form-build #1731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Generate minified CSS for frontend-subscriptions and ai-form-build #1731
Conversation
WalkthroughThis PR introduces a new Tailwind CSS minification workflow to the build pipeline, adds subscription stylesheet styling and enqueuing, updates frontend asset paths, removes filename uniqueness logic from uploads, and simplifies CSS asset path handling in the build configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Gruntfile.js (1)
340-353: Consider matching the existingtailwindtask pattern for consistency.The task works correctly but doesn't follow the same pattern as the existing
tailwindtask (lines 322-338), which usesthis.async()anddone(). While both approaches work withgrunt.task.run(), consistency would improve maintainability.Apply this diff for consistency:
grunt.registerTask('tailwind-minify', function() { + const done = this.async(); + const cssFiles = [ { input: 'assets/css/forms-list.css', output: 'assets/css/forms-list.min.css' }, { input: 'assets/css/frontend-subscriptions.css', output: 'assets/css/frontend-subscriptions.min.css' }, { input: 'assets/css/ai-form-builder.css', output: 'assets/css/ai-form-builder.min.css' }, { input: 'assets/css/admin/subscriptions.css', output: 'assets/css/admin/subscriptions.min.css' } ]; cssFiles.forEach(file => { if (grunt.file.exists(file.input)) { grunt.task.run(`shell:tailwind_minify:${file.input}:${file.output}`); } }); + + done(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
assets/js/ai-form-builder.min.jsis excluded by!**/*.min.jsassets/js/forms-list.min.jsis excluded by!**/*.min.jsassets/js/forms-list.min.js.mapis excluded by!**/*.map,!**/*.min.js.mapassets/js/frontend-subscriptions.min.jsis excluded by!**/*.min.jsassets/js/subscriptions.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (7)
Gruntfile.js(3 hunks)assets/js/frontend-subscriptions.js(1 hunks)includes/Ajax/Upload_Ajax.php(1 hunks)includes/Frontend.php(1 hunks)package.json(1 hunks)src/css/frontend-subscriptions.css(1 hunks)vite.config.mjs(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/css/frontend-subscriptions.css
[error] 82-82: expected } but instead the file ends
the file ends here
(parse)
🪛 GitHub Check: Run PHPCS inspection
includes/Ajax/Upload_Ajax.php
[failure] 292-292:
Whitespace found at end of line
🔇 Additional comments (6)
assets/js/frontend-subscriptions.js (1)
2-2: Import path aligns with built assets.Thanks for pointing the bundle at css/frontend-subscriptions.css; this matches the generated asset layout.
vite.config.mjs (1)
23-25: Unified CSS asset naming looks good.Standardizing on css/[name].min.css simplifies the release bundle and lines up with the Tailwind minification step.
includes/Frontend.php (1)
71-73: The style and script handles are properly registered.The
wpuf-frontend-subscriptionshandle is correctly registered through the Assets class. In includes/Assets.php, theget_styles()andget_scripts()methods define 'frontend-subscriptions' entries with proper src paths (lines 113 and 407). The constructor hooksregister_all_scripts()to the 'init' action, which registers all assets using the 'wpuf-' prefix, producing the final handle 'wpuf-frontend-subscriptions' that matches the enqueue calls in Frontend.php.Likely an incorrect or invalid review comment.
package.json (1)
10-15: LGTM! CSS build integration is correct.The build script properly chains the new CSS minification step, and the
build:cssscript correctly invokes the Grunt task. The trailing comma on line 14 follows good practice for maintainability.Gruntfile.js (2)
283-287: LGTM! Shell task configuration is correct.The
tailwind_minifyshell task properly uses the Tailwind CLI--minifyflag and follows the same pattern as the existingtailwindshell task.
313-313: LGTM! Release pipeline properly updated.The
tailwind-minifytask is correctly added to the release pipeline, ensuring minified CSS is generated during releases.
includes/Ajax/Upload_Ajax.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trim trailing whitespace to satisfy PHPCS.
Line 292 still has trailing spaces, which is tripping the PHPCS check (Whitespace found at end of line). Please strip the extra spaces.
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 292-292:
Whitespace found at end of line
🤖 Prompt for AI Agents
In includes/Ajax/Upload_Ajax.php around line 292, remove the trailing whitespace
characters at the end of line 292 (strip any spaces/tabs after the last visible
character) and save the file; also scan the file for any other trailing
whitespace and remove those to satisfy PHPCS.
| .subscription-template-classes { | ||
| @apply wpuf-rounded-xl wpuf-p-6 wpuf-ring-1 wpuf-ring-gray-200 wpuf-bg-white wpuf-shadow-md hover:wpuf-shadow-lg wpuf-transition-all wpuf-duration-300 wpuf-relative wpuf-h-full wpuf-flex wpuf-flex-col wpuf-m-2 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the missing semicolon after @apply.
The @apply in .subscription-template-classes is missing its terminating ;, which is why Biome reports expected } but instead the file ends and the stylesheet fails to compile. Please add the semicolon so Tailwind can process the file.
.subscription-template-classes {
- @apply wpuf-rounded-xl wpuf-p-6 wpuf-ring-1 wpuf-ring-gray-200 wpuf-bg-white wpuf-shadow-md hover:wpuf-shadow-lg wpuf-transition-all wpuf-duration-300 wpuf-relative wpuf-h-full wpuf-flex wpuf-flex-col wpuf-m-2
+ @apply wpuf-rounded-xl wpuf-p-6 wpuf-ring-1 wpuf-ring-gray-200 wpuf-bg-white wpuf-shadow-md hover:wpuf-shadow-lg wpuf-transition-all wpuf-duration-300 wpuf-relative wpuf-h-full wpuf-flex wpuf-flex-col wpuf-m-2;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .subscription-template-classes { | |
| @apply wpuf-rounded-xl wpuf-p-6 wpuf-ring-1 wpuf-ring-gray-200 wpuf-bg-white wpuf-shadow-md hover:wpuf-shadow-lg wpuf-transition-all wpuf-duration-300 wpuf-relative wpuf-h-full wpuf-flex wpuf-flex-col wpuf-m-2 | |
| } | |
| .subscription-template-classes { | |
| @apply wpuf-rounded-xl wpuf-p-6 wpuf-ring-1 wpuf-ring-gray-200 wpuf-bg-white wpuf-shadow-md hover:wpuf-shadow-lg wpuf-transition-all wpuf-duration-300 wpuf-relative wpuf-h-full wpuf-flex wpuf-flex-col wpuf-m-2; | |
| } |
🤖 Prompt for AI Agents
In src/css/frontend-subscriptions.css around lines 7 to 9, the @apply rule
inside the .subscription-template-classes block is missing its terminating
semicolon; add a semicolon at the end of the @apply declaration so the rule
reads as a complete statement and the CSS/Tailwind parser can properly compile
the file.
| return `npx tailwindcss -i ${input} -o ${output}`; | ||
| } | ||
| }, | ||
| tailwind_minify: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use existing uglify grunt tasks for minification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tailwind_minify shell command is defined at Gruntfile.js:283.
It plays a critical role in the asset optimization workflow for the project’s CSS files.
⚙️ Purpose
The command’s main function is to minify CSS files generated by Tailwind using the built-in --minify flag.
It ensures all Tailwind-processed CSS files are optimized before being deployed.
🔍 Why It’s Needed
1. Different Functional Scope
-
uglifytask → Handles JavaScript minification only. -
tailwind_minifytask → Handles CSS minification using Tailwind CLI.
Removing tailwind_minify would leave CSS unminified, negatively affecting bundle size and performance.
| "build:frontend-subscriptions": "ENTRY=frontend-subscriptions vite build", | ||
| "build:ai-form-builder": "ENTRY=ai-form-builder vite build" | ||
| "build:ai-form-builder": "ENTRY=ai-form-builder vite build", | ||
| "build:css": "grunt tailwind-minify" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align with Gruntfile.js [check if tailwind-minify is needed]
Close issue
Fix: Generate minified CSS for frontend-subscriptions and ai-form-builder
Now 'npm run build' generates both JS and CSS minified files in one command:
Files modified:
Summary by CodeRabbit
New Features
Changes
Chores